Skip to content

K9WebViewClient: log WebView main-frame load errors#11030

Merged
wmontwe merged 2 commits into
thunderbird:mainfrom
jim-daf:webview-onreceivederror-logging
May 26, 2026
Merged

K9WebViewClient: log WebView main-frame load errors#11030
wmontwe merged 2 commits into
thunderbird:mainfrom
jim-daf:webview-onreceivederror-logging

Conversation

@jim-daf
Copy link
Copy Markdown
Contributor

@jim-daf jim-daf commented May 20, 2026

Closes #11029.

K9WebViewClient already overrides shouldOverrideUrlLoading, shouldInterceptRequest and onPageFinished. It does not override onReceivedError, so any failure while loading a message body in the legacy message view is silent: the user sees a blank message body, and nothing useful gets recorded in adb logcat.

This PR adds the API 23+ overload only, since minSdk for the legacy module is 23 (build-plugin/src/main/kotlin/ThunderbirdProjectConfig.kt: const val sdkMin = 23), so the deprecated pre-M overload is not reachable on any device the app ships to.

@RequiresApi(Build.VERSION_CODES.M)
override fun onReceivedError(view: WebView, request: WebResourceRequest, error: WebResourceError) {
    super.onReceivedError(view, request, error)
    if (request.isForMainFrame) {
        Log.d("Message WebView load error: %d %s for %s", error.errorCode, error.description, request.url)
    }
}

Notes on scope:

  • Only main-frame errors are logged. Sub-resource errors are very common in HTML mails (broken remote images, blocked http:// resources inside an https:// mail) and would just spam the log without giving useful information about the root render failure.
  • The log call uses Log.d via net.thunderbird.core.logging.legacy.Log, matching the existing imports and call style in this file (see the existing Log.d(e, "Couldn't open URL: %s", uri) on the same class).
  • No new permission, no new dependency, no behaviour change for successful loads. The change is additive: an existing silent failure now produces a debug log line.

Out of scope but mentioned in the issue: QuotedMessageMvpView has the same gap. Happy to send a follow-up PR if maintainers prefer that one bundled, but K9WebViewClient is the dominant codepath and felt like the right place to start.

K9WebViewClient is the WebViewClient used to render email message bodies
in the legacy message view. It currently overrides shouldOverrideUrlLoading
and shouldInterceptRequest, but not onReceivedError. When the WebView fails
to load the main frame, the failure is swallowed and the user is left with
a blank message body. The error reason is not in the device log either,
which makes triage difficult.

Add the API 23+ onReceivedError override, scoped to request.isForMainFrame
so that occasional sub-resource errors (broken inline images, blocked
http content in https mails, etc.) do not flood the log. minSdk for the
legacy module is 23, so the deprecated pre-M overload is intentionally
not added.

No behaviour change for successful loads; on failure, the error code,
description, and failing URL are recorded via the legacy Log helper that
is already used elsewhere in this file.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

Validation Passed: All report and feature-flag labels are correctly set.

wmontwe
wmontwe previously approved these changes May 22, 2026
Copy link
Copy Markdown
Member

@wmontwe wmontwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@wmontwe wmontwe requested review from wmontwe and removed request for rafaeltonholo May 22, 2026 12:37
@wmontwe wmontwe assigned wmontwe and unassigned rafaeltonholo May 22, 2026
@wmontwe wmontwe added the report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). label May 22, 2026
@wmontwe
Copy link
Copy Markdown
Member

wmontwe commented May 22, 2026

@jim-daf There is a warning about too many methods, this could safely be ignored for this class by adding @Suppress("TooManyFunctions")to the class header.

@jim-daf
Copy link
Copy Markdown
Contributor Author

jim-daf commented May 22, 2026

@jim-daf There is a warning about too many methods, this could safely be ignored for this class by adding @Suppress("TooManyFunctions")to the class header.

Thanks @wmontwe, done in 5ad43d2. Added @Suppress("TooManyFunctions") on the class header, right under the existing KDoc.

Copy link
Copy Markdown
Member

@wmontwe wmontwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@wmontwe wmontwe merged commit 822704a into thunderbird:main May 26, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

K9WebViewClient swallows WebView load errors

3 participants